Time dependent QDM #226
Merged
Time dependent QDM #226
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A masking for measurements outside the intended time window of interest.
A masking for measurements outside the intended time window of interest. Building it directly in the standard QDM method.
A requirements to estimate parameters by time window.
This allows monthly quantiles or any other scales.
Expects the CDFs were estimated within time windows and apply the QDM correction respecting those.
This is used to support applying filters and masks.
A different direction. Instead of that, I generatlized the standard QuantileDeltaMappingCorrection to deal with N periods, which N=1 is the same behavior of the original function.
Using named arguments allow more freedom on the functions signatures thus more freedom for refactoring. Here we initiated the requirement on time to be able to apply QDM on different time scales, such as seasonally.
Merged
|
@grantbuster @bnb32 , whenever you have a chance, could you review this PR, please? It will make easier for you to review #215. This PR covers the time dependent solution for QDM. Instead of monthly it is generalized for any time interval. If we use 24 intervals with 30 day time window it would give us still the monthly scale but one estimate every 15 days, which would minimize the jump between on month transitions. Also, with that we avoid having to interpolate when applying and using instead the nearest value. More intervals we add, higher resolution, with an extreme of 365 intervals would lead to one CDF for each day of the year. |
grantbuster
requested changes
Jul 17, 2024
As requested by @grantbuster
bnb32
requested changes
Jul 17, 2024
As requested by @grantbuster
In case of a resolution higher than daily, it would make sense using the window extent without rounding it. Also fix a sign when after 365.
|
@grantbuster @bnb32 , I'm sorry I forgot to ping you yesterday. I believe that I covered all the requests. Please let me know if it is ready to merge. |
grantbuster
approved these changes
Jul 18, 2024
|
@castelao looks good thanks for implementing this! |
bnb32
approved these changes
Jul 18, 2024
github-actions bot
pushed a commit
that referenced
this pull request
Jul 18, 2024
* Initializing WindowedQuantileDeltaMappingCorrection * feat: window_mask() method A masking for measurements outside the intended time window of interest. * feat: window_mask() method directly on QDM A masking for measurements outside the intended time window of interest. Building it directly in the standard QDM method. * refact: run_single including bias time index A requirements to estimate parameters by time window. * feat: Quantiles estimated within moving time windows This allows monthly quantiles or any other scales. * refactor: local_qdm running on time windows Expects the CDFs were estimated within time windows and apply the QDM correction respecting those. * feat: Method to guide periods along a year This is used to support applying filters and masks. * doc: Info on assuming nearest available CDF * clean: Removing WindowedQuantileDeltaMappingCorrection A different direction. Instead of that, I generatlized the standard QuantileDeltaMappingCorrection to deal with N periods, which N=1 is the same behavior of the original function. * refactor: Using named arguments Using named arguments allow more freedom on the functions signatures thus more freedom for refactoring. Here we initiated the requirement on time to be able to apply QDM on different time scales, such as seasonally. * Adding ipython to dev environment * test: Adjusting tests to run with new temporal QDM * cfg: Updating pixi.lock * doc: window_mask() * doc, style: _window_center() * style: * style: * fix: Use nearest window center * Replacing variable t by idt As requested by @grantbuster * refact: Renaming variables As requested by @grantbuster * refact, fix: window_mask doesn't need rouding In case of a resolution higher than daily, it would make sense using the window extent without rounding it. Also fix a sign when after 365. * test: Testing window_mask() * doc: Improving window_mask() documentation * doc: Updating documentation to reflect new dimension on time window Issue noticed by @grantbuster * refact: Renaming time to time_index As requested by @bnb32. * doc: Information on the reference attributes As requested by @grantbuster.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
QDM was previously implemented to operate in a full time series, but PresRat requires seasonal CDFs. This was first achieved with a window sliding version of
bias.QuantileDeltaMappingCorrection, but later I generalized the standard QDM to run on any time periods. That satisfies the case ofn=1as it was before, as well asn=12which is approximately the monthly solution.The #215 already incorporated these modifications but for the sake of simplicity, this PR should be merged before #215.